Skip to content

Conversation

@tejlmand
Copy link
Contributor

@tejlmand tejlmand commented Jan 7, 2021

West has introduced support for group tags in:
zephyrproject-rtos/west#454

This means that manifest files might start containing groups.
Zephyr itself only requires west>=0.7.2 where groups are not supported
but other Zephyr based projects might start using the group feature.

When using a west version with group support, then only active projects
will be processed as Zephyr modules.

West versions without group support will consider all projects active.

Signed-off-by: Torsten Rasmussen [email protected]

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, just one nit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit hacky; you can compare the west.version.__version__ value against west 0.9. See _ensure_min_version() for examples of how to use packaging.version.parse() to make a high level version object which supports >= comparison.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I know.
But that won't allow for fetching west source code and install manually for testing this feature, as that version would be 0.8.99.

This way allows for testing the group feature with Zephyr by installing west manually.
As soon as west 0.9.0 is released, then this code will be updated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As soon as west 0.9.0 is released, then this code will be updated.

OK, so this is DNM for now until that happens, right? Adding the label.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As soon as west 0.9.0 is released, then this code will be updated.

OK, so this is DNM for now until that happens, right? Adding the label.

I was actually thinking to have it in master for others to use, but I guess if people build west themselves, then they also know how to apply this patch to Zephyr.

Also, I see you are preparing the 0.9.0 release, #31241, which happened after I opened this PR, so DNM is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally got around to look at this again :)

@mbolivar-nordic mbolivar-nordic added the DNM This PR should not be merged (Do Not Merge) label Jan 13, 2021
West has introduced support for group tags in:
zephyrproject-rtos/west#454

This means that manifest files might start containing groups.
Zephyr itself only requires west>=0.7.2 where groups are not supported
but other Zephyr based projects might start using the group feature.

When using a west version with group support, then only active projects
will be processed as Zephyr modules.

West versions without group support will consider all projects active.

Signed-off-by: Torsten Rasmussen <[email protected]>
@tejlmand tejlmand removed the DNM This PR should not be merged (Do Not Merge) label Jan 28, 2021
@carlescufi carlescufi added this to the v2.5.0 milestone Jan 29, 2021
@carlescufi
Copy link
Member

@nashif this should go in in order to properly support west 0.9.0 in Zephyr 2.5.

@nashif nashif merged commit 0cafde6 into zephyrproject-rtos:master Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants